Skip to content

Comments

feat(code_review): Validate before scheduling#108545

Open
armenzg wants to merge 10 commits intomasterfrom
cursor/CW-837-event-payload-validation-a4c0
Open

feat(code_review): Validate before scheduling#108545
armenzg wants to merge 10 commits intomasterfrom
cursor/CW-837-event-payload-validation-a4c0

Conversation

@armenzg
Copy link
Member

@armenzg armenzg commented Feb 19, 2026

Adds pre-scheduling validation for event_payload within schedule_task to prevent invalid tasks from being queued.

This addresses issues where model changes could lead to tasks being scheduled with incompatible payloads when task brokers are updated before Sentry workers, causing failures downstream. Validation now occurs before the Celery task is dispatched, catching schema mismatches early and improving system robustness.


Linear Issue: CW-837

Open in Cursor Open in Web

Adds validation to schedule_task() to catch schema mismatches early,
before tasks are scheduled to the task broker. This prevents task
failures when model changes are deployed to task brokers before
Sentry workers are updated.

Changes:
- Validate transformed event payload using Pydantic models before
  scheduling the Celery task
- Use appropriate model (SeerCodeReviewTaskRequestForPrClosed or
  SeerCodeReviewTaskRequestForPrReview) based on request_type
- Log validation failures with detailed context
- Record INVALID_PAYLOAD metric when validation fails
- Add tests to verify validation happens before task scheduling

Fixes: CW-837

Co-authored-by: Armen Zambrano G. <armenzg@users.noreply.github.com>
@cursor
Copy link
Contributor

cursor bot commented Feb 19, 2026

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@linear
Copy link

linear bot commented Feb 19, 2026

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 19, 2026
@armenzg armenzg self-assigned this Feb 23, 2026
@armenzg armenzg changed the title Event payload validation feat(code_review): Validate before scheduling Feb 23, 2026
cursoragent and others added 2 commits February 23, 2026 15:55
Removes validation logic from process_github_webhook_event task since
payload is now validated before scheduling in schedule_task().

This prevents double validation and ensures the validation happens
at the right layer - before tasks are queued to the broker rather
than during task execution.

Changes:
- Remove Pydantic validation from process_github_webhook_event
- Move enum key conversion to schedule_task after validation
- Update docstring to clarify payload is pre-validated
- Simplify task execution to just forward payload to Seer

Co-authored-by: Armen Zambrano G. <armenzg@users.noreply.github.com>
Remove extra context from validation failure log statement
to keep logging concise.

Co-authored-by: Armen Zambrano G. <armenzg@users.noreply.github.com>
- Integrate tags parameter added in master
- Keep pre-scheduling validation logic
- Remove duplicate validation from task execution
- Revert unrelated changes to pull_request.py and issue_comment.py

Co-authored-by: Armen Zambrano G. <armenzg@users.noreply.github.com>
@armenzg armenzg requested a review from vaind February 23, 2026 16:01
@armenzg armenzg marked this pull request as ready for review February 23, 2026 16:02
@armenzg armenzg requested a review from a team as a code owner February 23, 2026 16:02
)
return

# Validate payload before scheduling to catch schema mismatches early
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validation is now happening within schedule_task before we schedule the task, thus, model changes and payload changes happen cannot be out of date.

Remove unused 'e' variable from exception handler to fix linting.

Co-authored-by: Armen Zambrano G. <armenzg@users.noreply.github.com>
cursoragent and others added 2 commits February 23, 2026 16:19
Remove tests that expected validation errors during task execution,
since validation now happens before scheduling. Validation at the
scheduling layer is already covered by tests in test_pull_request.py.

Co-authored-by: Armen Zambrano G. <armenzg@users.noreply.github.com>
Address Bugbot concerns:
1. Catch only pydantic.ValidationError instead of all exceptions
   to avoid hiding unexpected failures
2. Add datetime serialization in convert_enum_keys_to_strings()
   to ensure Celery can serialize all payload values

Co-authored-by: Armen Zambrano G. <armenzg@users.noreply.github.com>
Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

orjson.dumps() in make_seer_request() already handles datetime
serialization automatically. Converting datetime to strings early
breaks tests that expect datetime objects in the payload.

Co-authored-by: Armen Zambrano G. <armenzg@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants